Skip to content

fix(desktop): bump @lydell/node-pty to 1.2.0-beta.12#29803

Merged
Hona merged 2 commits into
anomalyco:devfrom
Hona:fix/desktop-node-pty-bump
May 29, 2026
Merged

fix(desktop): bump @lydell/node-pty to 1.2.0-beta.12#29803
Hona merged 2 commits into
anomalyco:devfrom
Hona:fix/desktop-node-pty-bump

Conversation

@Hona
Copy link
Copy Markdown
Member

@Hona Hona commented May 28, 2026

Summary

Bumps @lydell/node-pty (and its per-platform packages) from 1.2.0-beta.101.2.0-beta.12 to fix the repeated Windows desktop sidecar crash / stderr flood reported in #29599.

Root cause (reproduced headlessly on Windows, node v24.14.1)

The desktop sidecar resolves @lydell/node-pty to @lydell/node-pty-win32-x64 (via electron.vite.config.ts). On every PTY kill(), node-pty's WindowsPtyAgent does child_process.fork('conpty_console_list_agent'), and in beta.10 that agent calls getConsoleProcessList(shellPid) with no error handling:

// @lydell/node-pty-win32-x64@1.2.0-beta.10/lib/conpty_console_list_agent.js
var consoleProcessList = getConsoleProcessList(shellPid); // line 13 — throws uncaught

kill() forks this agent and then synchronously kills the pty, so by the time the agent runs AttachConsole(shellPid) the shell is already gone (and the Electron utilityProcess has no console at all) → native throws Error: AttachConsole failed → the forked process exits with the stack dumped to stderr, which the desktop surfaces as sidecar stderr {...}. This matches the issue's "Error 1" line-for-line and fires on every terminal close (reproduced 40/40, including in a detached no-console process).

Fix

@lydell/node-pty@1.2.0-beta.12 carries the upstream fix that wraps the call in try/catch and guards innerPid <= 0:

// beta.12
var consoleProcessList = [];
if (shellPid > 0) {
  try { consoleProcessList = getConsoleProcessList(shellPid); }
  catch (_a) { consoleProcessList = []; } // AttachConsole can fail if the process already exited or is invalid.
}

Verified locally: after the bump, spawning + killing a PTY produces no AttachConsole failed output.

Changes

  • package.json — catalog @lydell/node-pty1.2.0-beta.12
  • packages/desktop/package.json — the 6 @lydell/node-pty-<platform> optional deps → 1.2.0-beta.12
  • bun.lock — regenerated
  • packages/opencode/src/pty/pty.node.ts — drop the now-redundant @ts-expect-error on the @lydell/node-pty import (beta.12 ships node-pty.d.ts, so the directive became unused → TS2578)

Notes / not addressed here

  • The same forked agent still flashes a console window per terminal close (node-pty's fork has no windowsHide); beta.12 reduces but doesn't fully remove this.
  • This bump stops the crash/stderr flood. Sidecar supervision/auto-restart (so a sidecar crash doesn't leave the renderer permanently on net::ERR_FAILED / "fetch failed") is a separate follow-up in packages/desktop/src/main/index.ts.

Refs #29599

Hona added 2 commits May 29, 2026 09:28
Fixes the Windows sidecar crash flood where killing a PTY forks conpty_console_list_agent.js, which called getConsoleProcessList() without a try/catch and threw an uncaught 'AttachConsole failed' (node v24). beta.12 wraps the call in try/catch and guards innerPid<=0, eliminating the per-terminal-close crash.
beta.12 ships node-pty.d.ts, so the import now type-resolves and the suppression directive is flagged unused (TS2578).
Copilot AI review requested due to automatic review settings May 28, 2026 23:32
@Hona Hona requested a review from adamdotdevin as a code owner May 28, 2026 23:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Bumps @lydell/node-pty from 1.2.0-beta.10 to 1.2.0-beta.12 to pick up the upstream fix that wraps getConsoleProcessList in try/catch, eliminating the repeated Windows sidecar crash on PTY kill() (issue #29599). The now-redundant @ts-expect-error directive is removed since beta.12 ships type definitions.

Changes:

  • Bump catalog and all 6 platform-specific @lydell/node-pty-* optional deps to 1.2.0-beta.12 (with regenerated bun.lock).
  • Drop the @ts-expect-error directive above the @lydell/node-pty import in the Node PTY adapter.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

File Description
package.json Catalog @lydell/node-pty bumped to 1.2.0-beta.12.
packages/desktop/package.json All six platform-specific optional deps bumped to 1.2.0-beta.12.
bun.lock Regenerated lockfile entries and checksums for beta.12.
packages/opencode/src/pty/pty.node.ts Removes unused @ts-expect-error since beta.12 ships type defs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants